-
Notifications
You must be signed in to change notification settings - Fork 347
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(cli): ssh based auth with external key #1208
fix(cli): ssh based auth with external key #1208
Conversation
cmd/agent/workspace/up.go
Outdated
@@ -432,7 +432,7 @@ func CloneRepository(ctx context.Context, sshkey string, local bool, workspaceDi | |||
|
|||
// check if command exists | |||
if !command.Exists("git") { | |||
if local { | |||
if workspaceInfo.Agent.Local == "true" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how is workspaceInfo
populated? Do we need to concern ourseleves with nil checking Agent
or CLIOptions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I assume this may be outside the scope of this PR but why is true a string? Does a user set this? If so maybe we should cast to lowercase during the check in case of "True"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
workspaceInfo
is being passed along from the main DevPod binary, we'll error out way earlier if it is nil
. As for workspaceInfo.Agent
, it's not a pointer so we should be good.
The local
option can both be defined by provider authors in provider.yaml
and in code which is why it's of type StrBool
. We could implement a ToBool
method on it to make it safer to check, wyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK makes sense. Yes i think if the value comes from plain text we should either cast to a bool as you say or at least make case insensitive :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a Bool
function to types.StrBool
proxy git ssh traffic through our internal client because openssh might not be installed in the target environment. If we detect an external SSH key, we pass the GIT_SSH_COMMAND to git, pointing to the new `devpod helper ssh-git-clone` command.
9890ac1
to
1d832d4
Compare
cmd/agent/workspace/up.go
Outdated
@@ -432,7 +432,7 @@ func CloneRepository(ctx context.Context, sshkey string, local bool, workspaceDi | |||
|
|||
// check if command exists | |||
if !command.Exists("git") { | |||
if local { | |||
if workspaceInfo.Agent.Local == "true" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK makes sense. Yes i think if the value comes from plain text we should either cast to a bool as you say or at least make case insensitive :)
proxy git ssh traffic through our internal client because openssh might not
be installed in the target environment.
If we detect an external SSH key, we pass the GIT_SSH_COMMAND to git,
pointing to the new
devpod helper ssh-git-clone
command.